Skip to content

Conversation

@meln5674
Copy link
Contributor

Presently, attempting to rename a non-local (e.g. Oauth2 or LDAP) user results in an error, even if the requester is an administrator. As far as I can tell, this is a security feature, not architectural in nature, as automatic account linking could be used to take control of another user's account. This is not a concern for an administrator, who we should trust to know what they are doing.

This patch allows admins, and only admins, to rename non-local users.

Fixes #18308 (sort of)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 17, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Nov 17, 2025
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to add a test to cover the new behavior

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 17, 2025
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, given the following:
I think the second reason why this was disabled is also that it leads to desync between online data and the local data.
I am not sure if or when data is synced, and what will happen in this case.
Additionally, this means there is no way to reset it back to the online state.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 17, 2025
@lunny lunny added this to the 1.26.0 milestone Nov 17, 2025
@meln5674
Copy link
Contributor Author

It's better to add a test to cover the new behavior

Can do, will take a look at that tomorrow.

@wxiaoguang wxiaoguang marked this pull request as draft November 22, 2025 08:06
@meln5674 meln5674 force-pushed the feat/admin-rename-nonlocal-user branch 2 times, most recently from e38a773 to 77cef7e Compare November 22, 2025 21:20
@meln5674 meln5674 force-pushed the feat/admin-rename-nonlocal-user branch from 77cef7e to ffdd25e Compare November 22, 2025 21:38
@meln5674
Copy link
Contributor Author

Added tests, rebased, and squashed

@meln5674 meln5674 marked this pull request as ready for review November 22, 2025 21:39
@wxiaoguang wxiaoguang force-pushed the feat/admin-rename-nonlocal-user branch from 123f12e to d4fff82 Compare November 23, 2025 03:21
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 23, 2025
@lunny lunny enabled auto-merge (squash) November 23, 2025 20:32
@lunny lunny merged commit 688430e into go-gitea:main Nov 23, 2025
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't change username from non-local users

5 participants